-
Notifications
You must be signed in to change notification settings - Fork 276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue #71: Group #265
Issue #71: Group #265
Conversation
…ll Command timeout to 15 mins, remove app-manager`s intermediate state`s timeout
…t direct conversion
…vice_version = "default"
…cannot trigger the device-config updating's bug in some cases
… immediately during the check loop
# Conflicts: # src/Cargo.lock # src/component/cyfs-base/src/objects/any.rs # src/component/cyfs-lib/src/lib.rs # src/meta/cyfs-meta/src/creator.rs # src/tools/desc-tool/src/modify.rs
@@ -10,6 +10,49 @@ message StorageBodyContent { | |||
bytes value = 1; | |||
} | |||
|
|||
// ObjectShellObject | |||
message ObjectShellDescContent { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's about: #219
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry, I have commit it with the feature #71 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be a corresponding English readme
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'll translate it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now the group mechanism has not designed a proper acl?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No acl now. and it need a new discussion.
handler.clone(), | ||
)); | ||
|
||
server.at("group/push-proposal").put(Self::new( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be "/group/push-proposal"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The http protocol of rcp is recommended to use RESTFUL, not to have actions on the path, but to use method to identify, so the two paths of group will be better designed as the following
POST /group/service
POST /group/proposal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you, I will update it.
@@ -127,6 +127,40 @@ impl DeviceInfoManagerImpl { | |||
} | |||
} | |||
|
|||
async fn verfiy_group_own_signs( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The so-called "checksign" looks like it's just comparing hash values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I will rename it.
} | ||
} | ||
|
||
async fn put_object(&self, dec_id: &ObjectId, obj: NONObjectInfo) -> BuckyResult<()> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just use sync_std::future::timeout instead of relying on the select of futures, which is obviously more complicated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to hold the life of the future to continue. and start a new future to retry at the same time. maybe the first future is slow only.
rpath: rpath.to_string(), | ||
}; | ||
|
||
self.encode_common_headers(NONAction::PutObject, &req_common, &mut http_req); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For group services NONAction
field are not unnecessary and can be removed
com_req.req_path.as_deref(), | ||
); | ||
|
||
http_req.insert_header(CYFS_API_LEVEL, com_req.level.to_string()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
api level is non/ndn relatively obsolete design, for the group this function this should not be necessary
} | ||
|
||
pub(crate) fn make_default_common(dec_id: ObjectId) -> NONOutputRequestCommon { | ||
NONOutputRequestCommon { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here it is recommended that group-related interfaces have a separate RequestCommon, even if it is completely consistent with NON's, it is better to redefine it
pub type GroupRequestCommon = NONRequestCommon;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, you are right, I will define a new structure, the usefull field is only dec_id
now. others will be add when it's used.
No description provided.